tests/test_comparison.py (1)

230-233: Nit: docstring mentions "without warning" but the test doesn't assert it.

The docstring says items() returns pairs "without warning", but the test body only compares equality. If the no-warning behavior is part of the contract (vs. the prior implementation that may have emitted one), wrap the call in warnings.catch_warnings() with simplefilter('error'), or drop the "without warnings" phrase from the docstring to avoid misleading future readers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_comparison.py` around lines 230 - 233, The test docstring claims
items() returns pairs "without warning" but the body doesn't assert that no
warnings occur; update the test_items function to enforce the no-warning
contract by calling comp.items() inside a warnings.catch_warnings() context and
set warnings.simplefilter('error') so any warning would fail the test (refer to
test_items, comp = fx.Comparison([...]), and the items() call on comp with
optimized_base and optimized_with_chp), or alternatively remove the "without
warning" phrase from the docstring if the no-warning behavior is not required.

flixopt/comparison.py (1)

239-254: Minor: keys()/values()/items() allocate a throwaway dict on every call.

Each invocation goes through self.flow_systems, which rebuilds a brand-new dict via dict(zip(self._names, self._systems, strict=True)). The returned view is only valid because that temporary dict is held alive by the view object itself — functionally fine, but you pay an O(n) dict construction for what should be cheap accessors, and successive calls return views backed by different dict instances (so identity comparisons between them won't hold).

Since _names and _systems are already the source of truth, you can avoid the allocation entirely — either by caching the dict or by returning lightweight iterables. If strict KeysView/ValuesView/ItemsView typing is important, cache on first access:

Proposed refactor

-    `@property`
-    def flow_systems(self) -> dict[str, FlowSystem]:
-        """Access underlying FlowSystems as a dict mapping name → FlowSystem."""
-        return dict(zip(self._names, self._systems, strict=True))
+    `@property`
+    def flow_systems(self) -> dict[str, FlowSystem]:
+        """Access underlying FlowSystems as a dict mapping name → FlowSystem."""
+        if self._flow_systems_cache is None:
+            self._flow_systems_cache = dict(zip(self._names, self._systems, strict=True))
+        return self._flow_systems_cache

(with a corresponding self._flow_systems_cache: dict[str, FlowSystem] | None = None in __init__).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@flixopt/comparison.py` around lines 239 - 254, The keys()/values()/items()
accessors currently call the flow_systems property which recreates a dict each
time via dict(zip(self._names, self._systems, strict=True)), causing O(n)
allocation on every call; change flow_systems to use a cached mapping (add
self._flow_systems_cache: dict[str, FlowSystem] | None = None in __init__) and
have flow_systems return the cached dict if present or build-and-store it once,
and ensure any mutating code that updates _names or _systems invalidates the
cache (set _flow_systems_cache = None) so the cached dict stays correct; this
preserves KeysView/ValuesView/ItemsView semantics while avoiding repeated
allocations.

pyproject.toml (1)

80-80: Consider removing the xarray<2026.3 constraint — linopy v0.6.6 (latest) has no upper bound.

The code change is sound as-is and scoped to dev installs. However, linopy's latest release (v0.6.6, March 2026) already specifies xarray>=2024.2.0 with no upper bound, suggesting the xarray 2026.3+ compatibility fix has shipped. The TODO can likely be resolved and the constraint removed in a follow-up.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyproject.toml` at line 80, The dependency pin "xarray<2026.3" in
pyproject.toml is now unnecessary because linopy v0.6.6 drops the upper bound;
remove the "<2026.3" upper bound from the xarray specification (i.e., replace
"xarray<2026.3" with simply "xarray" or "xarray>=<minimum-version>" as
appropriate) and delete the accompanying TODO comment so the dev install no
longer constrains xarray to <2026.3; update any related comment to reflect that
linopy v0.6.6 provides compatibility.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes